-
Notifications
You must be signed in to change notification settings - Fork 342
Added override of ChatMessageContent.ToString #485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Should we add something to make it clearer that this is for debugging purposes only? I feel like it could confuse users into doing things like:
|
|
Maybe we can return a string that is prefixed by: "DEBUG: ". |
| switch (part.Kind) | ||
| { | ||
| case ChatMessageContentPartKind.Text: | ||
| builder.Append(part.Text); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can potentially return an empty string, which is against the ToString guidelines.
| break; | ||
|
|
||
| default: | ||
| builder.Append("<unknown content kind>"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This should say "<unknown content part kind>" or simply "<unknown>"
No other ToString implementations do it.
I think it's fine if they do. In fact I think we should do it in our samples. I know you are trying to avoid people writing incorrect code when they have images and such, but all the code I saw written by others (and us) does completion.Content[0].Text, which is even less correct. |
But other
I disagree that this is less correct. It implies that the way you handle content is dependent on the specifics of your scenario. As much as we wished Content was simpler than a collection of random things, that is how the REST API is implemented, and I think we're doing users a disservice by hiding that from them. In reality, no customer should use I agree that we need to improve debuggability. What do you think about implementing the functionality here using a custom view for the debugger via |
To improve debugging experience